Skip to content

[SPT-1572] intro in macro#231

Draft
NullIsOne wants to merge 13 commits intodevelopfrom
feature/SPT-1476/macro
Draft

[SPT-1572] intro in macro#231
NullIsOne wants to merge 13 commits intodevelopfrom
feature/SPT-1476/macro

Conversation

@NullIsOne
Copy link
Collaborator

@NullIsOne NullIsOne commented Jun 19, 2023 •
edited
Loading

Что сделано?

  • Написан макрос Mutable добавляющий структуре
  • Подключен макро-таргет в package ReactiveDataComponents
  • Подключен макро-таргет в основной пример на Xcodegen
  • ...

Зачем это сделано?

Для освоения макросов и скрытия однотипного кода в компонентах.

На что обратить внимание?

  • В тестах expansion не срабатывает, хотя по месту использования макрос генерирует функции как и ожидается
  • Рабочие конфигурации
    • SPM project + Components + Macro
    • Xcodegen + Example + Macro (привычный workspace)
  • Нерабочие конфигурации
    • Cocoapods project
  • Code coverage был отключен из-за проблем с macro таргетом
  • Стоит попробовать понизить iOS хотя бы до 12 (минимально поддерживаемая версия в Xcode 15)
  • Надо еще написать макрос генерирующий тело структуры Property
  • ...

Как протестировать?

  • Тапнуть в примере на строчку "Table with custom components"
  • Пример не крашится и визуально такой же как в предыдущем ПР

@NullIsOne NullIsOne requested review from Golubeykov and Ikeret June 19, 2023 05:57
@NullIsOne NullIsOne self-assigned this Jun 19, 2023
@github-actions
Copy link

github-actions bot commented Jun 19, 2023 •
edited
Loading

Warnings
⚠️

The .yml file has been modified. Keep changelog.md up to date.
⚠️ Oops! We have found some issues. It's better to fix them to keep code clean

SwiftLint found issues

Severity File Reason
Warning Decl+Extensions.swift:37 Lines should not have trailing whitespace. (trailing_whitespace)
Warning Decl+Extensions.swift:22 Limit vertical whitespace to a single empty line. Currently 2. (vertical_whitespace)
Warning BuildableMacro.swift:75 Force unwrapping should be avoided. (force_unwrapping)
Error BuildableMacro.swift:16 Line should be 145 characters or less: currently 277 characters (line_length)
Error BuildableMacro.swift:20 Line should be 145 characters or less: currently 231 characters (line_length)
Warning BuildableMacro.swift:51 Lines should not have trailing whitespace. (trailing_whitespace)
Error MutableMacro.swift:8 Line should be 145 characters or less: currently 231 characters (line_length)
Warning MutableMacro.swift:41 Lines should not have trailing whitespace. (trailing_whitespace)
Warning MessageView.swift:253 Line should be 145 characters or less: currently 151 characters (line_length)
Warning MessageView.swift:123 Lines should not have trailing whitespace. (trailing_whitespace)

Generated by 🚫 Danger Swift against 1a5c5ad

@NullIsOne NullIsOne added the 8.0 label Jun 19, 2023
@NullIsOne NullIsOne force-pushed the feature/SPT-1476/macro branch from 5a76a3e to 2251ecc Compare June 19, 2023 07:04
Golubeykov
Golubeykov reviewed Jun 19, 2023
View reviewed changes
Example/project.yml
options:
# Версия Xcode
xcodeVersion: 13.4.1
xcodeVersion: 15.0.0
Copy link
Contributor

@Golubeykov Golubeykov Jun 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Это что-то из локального сюда зашло? Как понимаю, только ты пробовал в XCode 15)

Copy link
Collaborator Author

@NullIsOne NullIsOne Jun 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

это намеренно)
макросы требуют Swift 5.9
Swift 5.9 только в Xcode 15

короче эта часть изменений будет долго висеть.

Golubeykov
Golubeykov reviewed Jun 19, 2023
View reviewed changes
Copy link
Contributor

@Golubeykov Golubeykov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Предлагаю добавить в описании, что это история для XCode 15) А то я не сразу понял, почему не запускается)

Ikeret
Ikeret previously approved these changes Jun 19, 2023
View reviewed changes
Copy link
Contributor

@Ikeret Ikeret left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

У меня все работает, сам макрос даже читаемый получился, но можно еще описание добавить в ///)

Base automatically changed from feature/SPT-1476/componets_model_builder to develop June 19, 2023 11:06
@NullIsOne NullIsOne dismissed Ikeret’s stale review June 19, 2023 11:06

The base branch was changed.

@NullIsOne
Copy link
Collaborator Author

NullIsOne commented Jun 20, 2023

Обновил MutableMacroTests
Теперь все кейсы теста корректно отрабатывают

@NullIsOne NullIsOne changed the title Feature/spt 1476/macro intro in macro Jun 20, 2023
@NullIsOne NullIsOne force-pushed the feature/SPT-1476/macro branch 3 times, most recently from efaeed3 to 838f392 Compare June 20, 2023 12:51
@NullIsOne
Copy link
Collaborator Author

NullIsOne commented Jun 20, 2023

Перевожу CI на Xcode 15 раз уж в Gihub Actions добавили его, а тут

{ platform:iOS, id:dvtdevice-DVTiPhonePlaceholder-iphoneos:placeholder, name:Any iOS Device, error:iOS 17.0 is not installed. To use with Xcode, first download and install the platform }

Не до конца установили (Не поставили симулятор)

Ikeret
Ikeret reviewed Jun 21, 2023
View reviewed changes
Components/Macro/Package.swift
import CompilerPluginSupport

let package = Package(
name: "Macro",
Copy link
Contributor

@Ikeret Ikeret Jun 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Какое то уникальное название надо придумать)

@NullIsOne NullIsOne force-pushed the feature/SPT-1476/macro branch from 838f392 to 8b9cee4 Compare July 20, 2023 11:49
@NullIsOne NullIsOne force-pushed the feature/SPT-1476/macro branch from 8b9cee4 to 81e0140 Compare July 25, 2023 11:20
@NullIsOne NullIsOne changed the title intro in macro [SPT-1572] intro in macro Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Ikeret Ikeret Ikeret left review comments

@Golubeykov Golubeykov Golubeykov left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@NullIsOne NullIsOne

Labels

8.0 enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@NullIsOne @Ikeret @Golubeykov